Automated Test: date-algorithm-enhanced #358
Conversation
* fix date override for fixed round robin + time zone in date override * check if slot is within working hours of fixed hosts * add test for date override in different time zone * fix date overrides for not fixed hosts (round robin) * code clean up * fix added test * use the correct timezone of user for date overrides --------- Co-authored-by: CarinaWolli <wollencarina@gmail.com>
📝 WalkthroughWalkthroughThe changes introduce timezone-aware support for availability scheduling. A new optional Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router as TRPC Router
participant Slots as Slots Logic
participant Availability as Availability<br/>Checks
Client->>Router: Request slots with eventType,<br/>schedule, users
Router->>Router: Derive organizerTimeZone from<br/>eventType/schedule/user
Router->>Slots: Generate available slots with<br/>organizerTimeZone
Slots->>Slots: Calculate timezone offsets for<br/>each override
Slots->>Slots: Derive inviteeOffset -<br/>organizerOffset delta
Slots->>Slots: Apply offset to override times<br/>in invitee UTC context
Slots-->>Router: Return computed slots
Router->>Router: Fetch dateOverrides and<br/>workingHours for availability
Router->>Availability: checkIfIsAvailable with<br/>organizerTimeZone, dateOverrides,<br/>workingHours
Availability->>Availability: Apply date override constraints<br/>with timezone alignment
Availability->>Availability: Check working hours fallback<br/>if no override matches
Availability-->>Router: Slot validity decision
Router-->>Client: Filtered available slots
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trpc/server/routers/viewer/slots.ts (1)
575-586:⚠️ Potential issue | 🟡 Minor
organizerTimeZonemay beundefinedwhenuserScheduleis not found.On line 579, if
userScheduleisundefined(no matching user found), thenorganizerTimeZonewill beundefined. ThecheckIfIsAvailablefunction typesorganizerTimeZoneas optional, so this won't crash, but the date-override and working-hours checks insidecheckIfIsAvailablewill silently fall back to a0offset (line 107), which may produce incorrect availability for organizers not in UTC.
🤖 Fix all issues with AI agents
In `@packages/trpc/server/routers/viewer/slots.ts`:
- Around line 137-151: The check for slot containment in workingHours
incorrectly computes end using slotStartTime, so start and end are identical;
update the predicate in the workingHours.find callback to compute end from
slotEndTime (use slotEndTime.hour()/minute()) instead of slotStartTime so the
comparison against workingHour.endTime correctly detects slots that extend past
working hours; ensure you only return true when either start <
workingHour.startTime or end > workingHour.endTime.
- Around line 102-135: The current logic using dateOverrides.find(...)
incorrectly compares dayjs objects with === and rejects slots if they fall
outside any single override; change it to iterate with dateOverrides.some(...)
to accept the slot if it is contained by at least one override for that day.
Inside the predicate use dayjs(...).isSame(...) instead of === to test same-day
equality (or compare formatted strings), compute utcOffset once, normalize
start/end with dayjs(date.start).add(utcOffset, "minutes") and
dayjs(date.end).add(utcOffset, "minutes"), then check containment with
.isSame(), .isBefore(), and .isAfter() against slotStartTime and slotEndTime (or
their normalized equivalents); set and use a boolean like dateOverrideExist only
to indicate at least one matching-day override was found, and return false only
if no matching override contains the slot (i.e., !someContains).
🧹 Nitpick comments (3)
packages/lib/slots.ts (1)
211-225:flatMapreturns a single object, not an array — usemapinstead.
flatMapis designed for callbacks that return arrays; here each callback returns a plain object, so it behaves identically tomapbut misleads readers into expecting array flattening.Also, the
.toString()calls onoverride.start(lines 212-213) are unnecessary —dayjs(override.start)already accepts aDateobject directly and is more reliable than parsing the locale-dependentDate.toString()output.Proposed cleanup
- const overrides = activeOverrides.flatMap((override) => { - const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset(); - const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset(); + const overrides = activeOverrides.map((override) => { + const organizerUtcOffset = dayjs(override.start).tz(override.timeZone).utcOffset(); + const inviteeUtcOffset = dayjs(override.start).tz(timeZone).utcOffset();packages/trpc/server/routers/viewer/slots.ts (2)
105-127: Bothfind()callbacks have code paths that don't return a value — usesome()instead.As flagged by Biome, the callbacks on lines 106 and 139 don't return a value on all paths (implicit
undefined). Since the result is only used as a boolean (truthy/falsy),some()is the correct method here and communicates intent more clearly.♻️ Replace find() with some()
- if ( - dateOverrides.find((date) => { + if ( + dateOverrides.some((date) => { ... + return false; }) ) {- if ( - workingHours.find((workingHour) => { + if ( + workingHours.some((workingHour) => { ... + return false; }) ) {Also applies to: 138-148
439-441: Reasonable fallback chain fororganizerTimeZone.The derivation
eventType.timeZone || eventType?.schedule?.timeZone || userAvailability?.[0]?.timeZoneprovides a sensible cascade. Note that if all three are falsy (unlikely but possible for misconfigured event types),organizerTimeZonewould beundefined, which would propagate asundefinedto downstream functions. Consider whether a final fallback (e.g.,"UTC") is warranted.
| //check if date override for slot exists | ||
| let dateOverrideExist = false; | ||
|
|
||
| if ( | ||
| dateOverrides.find((date) => { | ||
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | ||
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") | ||
| ) { | ||
| dateOverrideExist = true; | ||
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { | ||
| return true; | ||
| } | ||
| if ( | ||
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || | ||
| slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes")) | ||
| ) { | ||
| return true; | ||
| } | ||
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { | ||
| return true; | ||
| } | ||
| } | ||
| }) | ||
| ) { | ||
| // slot is not within the date override | ||
| return false; | ||
| } | ||
|
|
||
| if (dateOverrideExist) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Critical: Reference-equality comparison of dayjs objects on line 114 is always false.
dayjs(...) === dayjs(...) compares two distinct object references — this will never be true. Use .isSame() to compare dayjs values:
- if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) {
+ if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) {Additionally, the broader find()-based logic here is incorrect when multiple date overrides exist for the same day (e.g., two users with different override windows, or a user with morning + afternoon override blocks). The find() will match any override that the slot falls outside of, even if the slot is valid inside a different override. This causes valid slots to be rejected.
For example, with overrides A (10am–12pm) and B (2pm–4pm), a slot at 11am would be incorrectly rejected because it falls outside override B, even though it's inside override A.
Consider restructuring: if any override matches this day, the slot should be accepted if it falls within at least one of the matching overrides (i.e., use some() to check containment, not find() to check exclusion).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //check if date override for slot exists | |
| let dateOverrideExist = false; | |
| if ( | |
| dateOverrides.find((date) => { | |
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | |
| if ( | |
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | |
| slotStartTime.format("YYYY MM DD") | |
| ) { | |
| dateOverrideExist = true; | |
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { | |
| return true; | |
| } | |
| if ( | |
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || | |
| slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes")) | |
| ) { | |
| return true; | |
| } | |
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { | |
| return true; | |
| } | |
| } | |
| }) | |
| ) { | |
| // slot is not within the date override | |
| return false; | |
| } | |
| if (dateOverrideExist) { | |
| return true; | |
| } | |
| //check if date override for slot exists | |
| let dateOverrideExist = false; | |
| if ( | |
| dateOverrides.find((date) => { | |
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | |
| if ( | |
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | |
| slotStartTime.format("YYYY MM DD") | |
| ) { | |
| dateOverrideExist = true; | |
| if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) { | |
| return true; | |
| } | |
| if ( | |
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || | |
| slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes")) | |
| ) { | |
| return true; | |
| } | |
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { | |
| return true; | |
| } | |
| } | |
| }) | |
| ) { | |
| // slot is not within the date override | |
| return false; | |
| } | |
| if (dateOverrideExist) { | |
| return true; | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 106-106: This callback passed to find() iterable method should always return a value.
Add missing return statements so that this callback returns a value on all execution paths.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/slots.ts` around lines 102 - 135, The
current logic using dateOverrides.find(...) incorrectly compares dayjs objects
with === and rejects slots if they fall outside any single override; change it
to iterate with dateOverrides.some(...) to accept the slot if it is contained by
at least one override for that day. Inside the predicate use
dayjs(...).isSame(...) instead of === to test same-day equality (or compare
formatted strings), compute utcOffset once, normalize start/end with
dayjs(date.start).add(utcOffset, "minutes") and dayjs(date.end).add(utcOffset,
"minutes"), then check containment with .isSame(), .isBefore(), and .isAfter()
against slotStartTime and slotEndTime (or their normalized equivalents); set and
use a boolean like dateOverrideExist only to indicate at least one matching-day
override was found, and return false only if no matching override contains the
slot (i.e., !someContains).
| //if no date override for slot exists check if it is within normal work hours | ||
| if ( | ||
| workingHours.find((workingHour) => { | ||
| if (workingHour.days.includes(slotStartTime.day())) { | ||
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | ||
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); | ||
| if (start < workingHour.startTime || end > workingHour.endTime) { | ||
| return true; | ||
| } | ||
| } | ||
| }) | ||
| ) { | ||
| // slot is outside of working hours | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Bug: end is computed identically to start — slot end time is never validated against working hours.
Lines 141-142 both use slotStartTime, making start === end always true. The end variable should use slotEndTime:
if (workingHour.days.includes(slotStartTime.day())) {
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
- const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+ const end = slotEndTime.hour() * 60 + slotEndTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {This means a slot that starts within working hours but extends past workingHour.endTime would currently not be filtered out.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //if no date override for slot exists check if it is within normal work hours | |
| if ( | |
| workingHours.find((workingHour) => { | |
| if (workingHour.days.includes(slotStartTime.day())) { | |
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | |
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); | |
| if (start < workingHour.startTime || end > workingHour.endTime) { | |
| return true; | |
| } | |
| } | |
| }) | |
| ) { | |
| // slot is outside of working hours | |
| return false; | |
| } | |
| //if no date override for slot exists check if it is within normal work hours | |
| if ( | |
| workingHours.find((workingHour) => { | |
| if (workingHour.days.includes(slotStartTime.day())) { | |
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); | |
| if (start < workingHour.startTime || end > workingHour.endTime) { | |
| return true; | |
| } | |
| } | |
| }) | |
| ) { | |
| // slot is outside of working hours | |
| return false; | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 139-139: This callback passed to find() iterable method should always return a value.
Add missing return statements so that this callback returns a value on all execution paths.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/slots.ts` around lines 137 - 151, The
check for slot containment in workingHours incorrectly computes end using
slotStartTime, so start and end are identical; update the predicate in the
workingHours.find callback to compute end from slotEndTime (use
slotEndTime.hour()/minute()) instead of slotStartTime so the comparison against
workingHour.endTime correctly detects slots that extend past working hours;
ensure you only return true when either start < workingHour.startTime or end >
workingHour.endTime.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
New Features
Improvements